fix(client): Enable loading different file types when running in parent mode without iframe #3289
Conversation
…g in parent mode without iframe Back in karma-runner#2542, a third option to run tests without iframe is implemented, mostly for lightweight browser. This fix is to include file type so files like .css would able to load properly.
…nt mode without iframe Back in karma-runner#2542, a third option to run tests without iframe is implemented, mostly for lightweight browser. It only allows script element to be loaded dynamically. This fix includes file type like .css to be loaded properly.
|
Updated the description with more detailed information. Please review. Thanks! |
client/karma.js
Outdated
| var ele = document.createElement('script') | ||
| ele.src = window.__karma__.scriptUrls[idx] | ||
| var parser = new DOMParser() | ||
| // Browsers don't like character <> in string when assigning |
There was a problem hiding this comment.
Rather than "inanimate object does not like" can we say eg
// Escape characters with special roles in HTML that appear in the body of tags.
But I don't understand this. How can a DOM parser not support angle bracket parsing?
There was a problem hiding this comment.
Let's say we have only one file (bar/foo.js). We then construct this element that would like to add to the dom (<script type="text/javascript" src="/bar/foo.js" ></script>). When we embed this string into %SCRIPT_URL_ARRAY% in client_with_context.html. The html file will have a line look like this: window.karma.scriptUrls = ["<script type="text/javascript" src="/bar/foo.js" ></script>"]. When the browser parse the html file, they stop parsing once it reaches the character "<". They parse it and expect it as an open tag even though it is wrapped with double quotation. This happens on both Chrome and the lightweight browser I am using. Therefore I turn them to hex code which looks like this in the html: window.karma.scriptUrls = ["\x3Cscript type="text/javascript" src="/bar/foo.js" \x3E\x3C/script\x3E"]
There was a problem hiding this comment.
Wouldn't it be clearer to have the template be an array of objects:
[{type: "text/javascript", src: "/bar/foo.js" }, {...}] Then in this code we walk the object and create the script tags. So no angle brackets in the template means no reason to encode then replace the angle brackets?
There was a problem hiding this comment.
I also thought about this approach too. However, the script tags html strings are constructed case by case in the middleware.
if (fileType === 'css') {
scriptTags.push(`<link type="text/css" href="${filePath}" rel="stylesheet">`)
} else if (fileType === 'dom') {
scriptTags.push(includedContent.get(file.path))
} else if (fileType === 'html') {
scriptTags.push(`<link href="${filePath}" rel="import">`)
} else {
const scriptType = (SCRIPT_TYPE[fileType] || 'text/javascript')
const crossOriginAttribute = includeCrossOriginAttribute ? 'crossorigin="anonymous"' : ''
scriptTags.push(`<script type="${scriptType}" src="${filePath}" ${crossOriginAttribute}></script>`)
}
}
If this changes in the future, we have to update the logic to construct and reconstruct the script tag object too. I think that actually adds another layer of complexity and maintenance. Therefore, I prefer just escaping and de-escaping the generated string here (which I think also have closest logic to the regular case).
client/karma.js
Outdated
| var ele = document.createElement('script') | ||
| ele.src = window.__karma__.scriptUrls[idx] | ||
| var parser = new DOMParser() | ||
| // Browsers don't like character <> in string when assigning |
There was a problem hiding this comment.
Wouldn't it be clearer to have the template be an array of objects:
[{type: "text/javascript", src: "/bar/foo.js" }, {...}] Then in this code we walk the object and create the script tags. So no angle brackets in the template means no reason to encode then replace the angle brackets?
Add conditions to check whether it is serving client_with_context.html to construct the scriptUrl Fixes karma-runner#3289
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
Fix CLA |
|
@googlebot Fix CLA |
client/karma.js
Outdated
| var tmp = ele | ||
| ele = document.createElement('script') | ||
| ele.src = tmp.src | ||
| ele.crossOrigin = tmp.crossorigin |
There was a problem hiding this comment.
Is the different case significant? If yes please add a comment; if no please pick one.
Enable loading different file types when running in parent mode without iframe.
By default, the Karma middleware reads the included files and generates the corresponding HTML in string which eventually replaced the %SCRIPTS%. The client then later loads the iframe that embeds this context.html.
In #2542, I introduced a third option to run tests on lightweight browsers without iframe support. This is achieved by running test within the parent page and dynamically load all the files. I created a client_with_context.html which combines both client.html and context.html. The Karma middleware will concatenate all the included file paths, turns it to a long string with ',' separator and replace it with %SCRIPT_URL_ARRAY% in client_with_context.html. On the client side, it will read the %SCRIPT_URL_ARRAY% string and dynamically load the file one by one by creating script element. This is where the bug is introduced because I assume that all the included files are javascript, which actually covers most of the use cases in Karma.
Recently, we would like to run Karma for UI testing using a screenshot plugin and we see css files are actually not properly loaded. In this fix, instead of passing the files paths to client, we are passing the HTML element in string. The client reads the HTML in string and created the corresponding html element using DOMParser() and then it is appended to the DOM dynamically.
When I try out this new approach, one of the issue I see is script elements generated by DOMParser don't execute. Therefore, we have to use document.createElement for script elements.